Skip to content

Added basic doc for dealing with DTO#369

Merged
dunglas merged 2 commits into
api-platform:masterfrom
lyrixx:dto
Dec 27, 2017
Merged

Added basic doc for dealing with DTO#369
dunglas merged 2 commits into
api-platform:masterfrom
lyrixx:dto

Conversation

@lyrixx

@lyrixx lyrixx commented Dec 26, 2017

Copy link
Copy Markdown
Contributor

Comment thread core/dto.md Outdated
}
```

In this case, we disable all operation exept `POST`.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

except

@dunglas dunglas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also make classes final please?

Comment thread core/dto.md Outdated

use ApiPlatform\Core\EventListener\EventPriorities;
use App\Manager\UserManager;
use Joli\Entity\User;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

App\Entity\User?

Comment thread core/dto.md Outdated

## How to use a DTO for writing

Sometimes it's easier to use a DTO than an Entity when performing simple that.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra "that" to remove?

Comment thread core/dto.md Outdated
@@ -0,0 +1,116 @@
# DTO - Data Transfer Object

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May I suggest: Handling Data Transfer Objects (DTOs)

Comment thread core/dto.md Outdated
@@ -0,0 +1,116 @@
# DTO - Data Transfer Object

## How to use a DTO for writing

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use title case for titles: How to use a DTO for Writing

Comment thread core/dto.md Outdated
## How to use a DTO for writing

Sometimes it's easier to use a DTO than an Entity when performing simple that.
For exemple, if the application should be able to send an email when someone has

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

example

Comment thread core/dto.md Outdated
}
```

In this case, we disable all operation exept `POST`.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

operations except

Comment thread core/dto.md Outdated

In this case, we disable all operation exept `POST`.

Then, thanks to the [the event system](events.md), it's possible to intercept

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

double the

Comment thread core/dto.md Outdated
public static function getSubscribedEvents()
{
return [
KernelEvents::VIEW => [

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC, you can remove the outer array.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes it's possible. But usually the listener can be re-used to handle many operation.
And it could avoid api-platform/core#1590 (comment)

Anyway, I removed the outer array

Comment thread core/dto.md Outdated
## How to use a DTO for writing

Sometimes it's easier to use a DTO than an Entity when performing simple that.
For exemple, if the application should be able to send an email when someone has

@alanpoulain alanpoulain Dec 26, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example, the application should be able to send an email when someone has lost its password.

Comment thread core/dto.md Outdated

## How to use a DTO for writing

Sometimes it's easier to use a DTO than an Entity when performing simple that.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that? :p

Comment thread core/dto.md Outdated
}
```

Then this class should be registred as a service, then tagged.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

registered

@lyrixx

lyrixx commented Dec 27, 2017

Copy link
Copy Markdown
Contributor Author

@alanpoulain & @dunglas Thanks for the review. I have addressed your comments.

@dunglas
dunglas merged commit 5cbe6b0 into api-platform:master Dec 27, 2017
@dunglas

dunglas commented Dec 27, 2017

Copy link
Copy Markdown
Member

Thanks @lyrixx! Great piece of docs.

@lyrixx
lyrixx deleted the dto branch December 27, 2017 11:13
@aitboudad

aitboudad commented Dec 27, 2017

Copy link
Copy Markdown
Contributor

added one missing step in #371

Comment thread core/dto.md
{
$request = $event->getRequest();

if ('api_forgot_password_requests_post_collection' !== $request->attributes->get('_route')) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't better to check the model instance instead ?

if (
    !Request::METHOD_POST === $request->getMethod()
    || !$event->getControllerResult() instanceof ForgotPasswordRequest
) {
    return;
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, Now I prefer to match the route name. This reduce side effect.

But anyway, It's not really perfect IMHO

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahh ok, IMO there is no perfect way ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants